Skip to content

feat: add strict typed identity model v2 - #85

Closed
Pigbibi wants to merge 2 commits into
mainfrom
codex/canonical-typed-identity-r1c
Closed

feat: add strict typed identity model v2#85
Pigbibi wants to merge 2 commits into
mainfrom
codex/canonical-typed-identity-r1c

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add the strict structured_tokens.v2 canonical identity model
  • keep credential recognition outside R1 and reserve secret_ref as the only credential representation
  • enforce exact canonical record and three-digest verification

Scope

Fresh R1c replacement for frozen PR #84. Pure model and tests only; no runtime, history, evidence, reviewer adapter, or R2 integration.

Validation

  • python3 -m unittest tests.test_canonical_typed_identity_r1c
  • python3 -m unittest tests.test_run_codex_pr_review
  • python3 -m unittest discover tests
  • python3 -m ruff check .
  • python3 -m compileall -q service scripts tests
  • actionlint .github/workflows/*.yml
  • git diff --check origin/main...HEAD

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

🚫 Merge blocked: 1 serious issue(s) found in high-risk files

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🟠 [HIGH] Logic in scripts/canonical_typed_identity.py

_clauses() only checks that each element is a syntactically valid token object; it never validates the clause grammar for predicates, required_behavior, forbidden_behavior, or ordering_constraints. As a result, records like a single operator, consecutive operators, or arbitrary secret_ref placements are accepted, hashed, and later pass verify_identity_record(). Any downstream consumer that assumes these fields are well-formed expressions can mis-evaluate them or crash on a supposedly verified identity. (line 100)

Suggestion: Add field-specific grammar validation before hashing. For example, require predicates to alternate operand/operator and end with an operand, constrain behavior clauses to the allowed token shapes, and define an explicit grammar for ordering_constraints instead of accepting any token sequence.


Review by Codex PR Review bot • PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 943a3ca345

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if kind == "secret_ref":
ref = _obj(token["value"], {"type", "role", "position"})
typ, role, position = ref["type"], ref["role"], ref["position"]
if typ not in SECRET_TYPES or role not in SECRET_ROLES or isinstance(position, bool) or not isinstance(position, int) or not 0 <= position <= 1024:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject non-string secret_ref fields consistently

When a JSON payload supplies a secret_ref whose type or role is an array/object, this membership test hashes the untrusted value and raises TypeError instead of the module’s IdentityError. That leaks an unexpected exception path from validate_identity/verify_identity_record for malformed identity data, unlike the surrounding validators; check these fields are bounded strings before testing them against the finite sets.

Useful? React with 👍 / 👎.

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by merged PR #87. This PR remains unmerged; its implementation is not imported.

@Pigbibi Pigbibi closed this Jul 13, 2026
@Pigbibi
Pigbibi deleted the codex/canonical-typed-identity-r1c branch July 13, 2026 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant